Issue 257: Support for OnlyAllowAdmins as a server startup config #297
Conversation
|
We don't use the admin too since we launch with a windows service, we would edit and set the |
|
The changes in the admin tool is mostly just "cosmetic" to improve log output, and can be removed entirely if you wish. It's not coupled with the actual feature.
I'm not sure I understand your suggestion. You'd like for the RelayStateService to be initialized with the value from the .ini config in the bootstrapper, instead of the propsed changes? |
Sorry, no was reading things too quickly and needed to remind myself how the ini inits the GlobalConfig. I see how you are using it in the relaystate to initialize it with its value on startup with the info builder. all good |
MikeJeffers
left a comment
MikeJeffers
left a comment
There was a problem hiding this comment.
All good, just realized what was actually happening between the server and admintool reading of the ini file.
|
|
||
| private bool GetAdminOnlyModeFromIni(string gameRoot) | ||
| { | ||
| var perpetuumIni = Path.Combine(gameRoot, PERPETUUMINIFILE); |
There was a problem hiding this comment.
The only nit I'll put here is that the ini file should be read once, by one service, so there is no potential for split/conflicting state to occur.
A very edge-y case for something that is just for logging on a tool we don't use, but still something to consider.
Problem becomes exposing things to the admin tool - and maybe thats messier or causes more violations of design principles.
There was a problem hiding this comment.
(sorry I meant to leave this in the original review, but didn't commit the comment :( )
There was a problem hiding this comment.
Nah, good catch! It's bad practice and should be pointed out.
20f674e refactors it, so we read the file once and put the values into a dictionary - and then read the values from the dict instead.
There was a problem hiding this comment.
Ah yes this, but I also mean that the server itself reads the ini file for GlobalConfig <- that's where the server's copy of this data resides.
TBD if it actually bad/nasty to expose this object to something like a client like the Admintool. I suspect yes, so I'm cool if basically we assume the state of the ini file will not change between Admintool and server boot
2 participants
This change introduces
"StartServerInAdminOnlyMode": true/falseas a key in thedata/perpetuum.inifile.If it's set to true, the server will start up with the
RelayState.OpenForAdminsOnlystate.This state can then be changed during runtime, by logging in on an admin game account and doing
#relayopenor#relayclose. This will not edit the perpetuum.ini file, only the currently running state.If the
StartServerInAdminOnlymodekey is missing from the .ini file, we default to false - Meaning the server will start up withRelayState.OpenForPublicThe already merged PR #290 + this PR = Closes #257
Test:
"StartServerInAdminOnlyMode": truein the .ini file#secureand#relayopenin a chatSanity:
"StartServerInAdminOnlyMode": false(or alternatively, remove the StartServerInAdminOnlyMode key completely)